fix(Android): update status bar & orientation in screen stack fragment #1934
Conversation
Status bar, Navigation bar and Orientation are changed in the trySetWindowTrait function, called in ScreenFragment that ScreenStackFragment extends - so do not forget the super to apply it correctly! This ensure status bar, navigation bar and orientation are set, even when Fragment or Activity are not available yet when the props is set, which can happen for example on the first setting of the Props. A similar fix is also done in the ScreenStackHeaderConfig onUpdate function.
|
For apps having a Removing it from ScreenStackHeaderConfig.onUpdate can't be done I think, because this function is called elsewhere - probably in a situation where the fix was needed! |
delphinebugner
changed the title
|
Hi @delphinebugner, thanks for submitting the PR! Really appreciate the time you've spent on debugging the lifecycle of the screens ❤️ I'll try to check the changes in upcoming days and will come back with the update 🤞 also,
Yeah, I believe I understand what do you mean about that - when I was implementing #1874, I've spotted an issue where This is quite problematic for us, because you can't just trust that top app bar will be created properly (at least in the mentioned above PR, where you need to provide additional native components, like AppBarLayout, CollapsingToolbarLayout and MaterialToolbar). I think this may be related to your case of double calling |
There was a problem hiding this comment.
At first glance it looks good to me, haven't spotted any bugs there! But I may just have wrong repro 😄 @delphinebugner could you attach your test application to this PR (or any snack / link to repo) so I could check if this change fixes your problem?
|
The change fixes my problem, I patched it locally already! But my app is on a private (enterprise) repository, I can't share the code with you 😥 |
|
@delphinebugner Sure! Then I would say I trust you, as I've tested by myself how our example apps behave 😄 I also believe that someone has just forgot about that |
tboba
changed the title
|
Hi @delphinebugner! I'm happy to say that we've released new version of react-native-screens (3.29.0) that contains changes from this PR 🥳 Check it out! If you find something wrong related to the newest version (this change is still buggy or doesn't work for you) let us know 🎉 |
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [react-native-screens](https://togithub.com/software-mansion/react-native-screens) | [`^3.27.0` -> `^3.29.0`](https://renovatebot.com/diffs/npm/react-native-screens/3.27.0/3.29.0) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>software-mansion/react-native-screens (react-native-screens)</summary> ### [`v3.29.0`](https://togithub.com/software-mansion/react-native-screens/releases/tag/3.29.0) [Compare Source](https://togithub.com/software-mansion/react-native-screens/compare/3.28.0...3.29.0) Minor release including fix for iOS that was accidentally omitted from 3.28.0. It should be now possible to present modal in outer stack, from modal in nested stack (😄 ) #### What's Changed #### 🐛 Bug fixes - fix(iOS): select correct VC for nested modal presentation by [@​kkafar](https://togithub.com/kkafar) in [https://github.com/software-mansion/react-native-screens/pull/1912](https://togithub.com/software-mansion/react-native-screens/pull/1912) **Full Changelog**: software-mansion/react-native-screens@3.28.0...3.29.0 ### [`v3.28.0`](https://togithub.com/software-mansion/react-native-screens/releases/tag/3.28.0) [Compare Source](https://togithub.com/software-mansion/react-native-screens/compare/3.27.0...3.28.0) Minor release adding a support for **React Native 0.73**, adding new iOS-like slide animation, fixing crashes with AVPlayer on iOS and resolving build issues on Android. #### What's Changed #### 🐛 Bug fixes - Update status bar & orientation in screen stack fragment by [@​delphinebugner](https://togithub.com/delphinebugner) in [https://github.com/software-mansion/react-native-screens/pull/1934](https://togithub.com/software-mansion/react-native-screens/pull/1934) - Set stateWrapper in ScreenViewManager in Fabric by [@​joemun](https://togithub.com/joemun) in [https://github.com/software-mansion/react-native-screens/pull/1944](https://togithub.com/software-mansion/react-native-screens/pull/1944) - Don't include AVPlayerView in `traverseForScrollView` method by [@​tboba](https://togithub.com/tboba) in [https://github.com/software-mansion/react-native-screens/pull/1969](https://togithub.com/software-mansion/react-native-screens/pull/1969) - Fix error about duplicate class ViewModelLazy by [@​tboba](https://togithub.com/tboba) in [https://github.com/software-mansion/react-native-screens/pull/1977](https://togithub.com/software-mansion/react-native-screens/pull/1977) - Move DelayedFreeze setImmediate into an effect by [@​amadeus](https://togithub.com/amadeus) in [https://github.com/software-mansion/react-native-screens/pull/1980](https://togithub.com/software-mansion/react-native-screens/pull/1980) #### 👍 Improvements - Add ios like slide animation by [@​alexandrius](https://togithub.com/alexandrius) in [https://github.com/software-mansion/react-native-screens/pull/1945](https://togithub.com/software-mansion/react-native-screens/pull/1945) #### 🔢 Miscellaneous - Support for RN 0.73 by [@​kkafar](https://togithub.com/kkafar) in [https://github.com/software-mansion/react-native-screens/pull/1956](https://togithub.com/software-mansion/react-native-screens/pull/1956) - Use JDK 17 for CI builds as required for RN 0.73 by [@​kkafar](https://togithub.com/kkafar) in [https://github.com/software-mansion/react-native-screens/pull/1957](https://togithub.com/software-mansion/react-native-screens/pull/1957) - Update Podfile.lock files in example projects by [@​tboba](https://togithub.com/tboba) in [https://github.com/software-mansion/react-native-screens/pull/1979](https://togithub.com/software-mansion/react-native-screens/pull/1979) #### New Contributors - [@​delphinebugner](https://togithub.com/delphinebugner) made their first contribution in [https://github.com/software-mansion/react-native-screens/pull/1934](https://togithub.com/software-mansion/react-native-screens/pull/1934) - [@​joemun](https://togithub.com/joemun) made their first contribution in [https://github.com/software-mansion/react-native-screens/pull/1944](https://togithub.com/software-mansion/react-native-screens/pull/1944) - [@​alexandrius](https://togithub.com/alexandrius) made their first contribution in [https://github.com/software-mansion/react-native-screens/pull/1945](https://togithub.com/software-mansion/react-native-screens/pull/1945) - [@​amadeus](https://togithub.com/amadeus) made their first contribution in [https://github.com/software-mansion/react-native-screens/pull/1980](https://togithub.com/software-mansion/react-native-screens/pull/1980) **Full Changelog**: software-mansion/react-native-screens@3.27.0...3.28.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 5pm,every weekend" in timezone America/Los_Angeles, Automerge - "after 5pm,every weekend" in timezone America/Los_Angeles. 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/valora-inc/wallet). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: valora-bot <[email protected]>
](https://renovatebot.com){kind=link}
software-mansion#1934) ## Description I found this working on my app going in portrait mode on Android quite aleatory, when it was supposed to be locked on React Navigation - using the `orientation: "portrait"` on my screens. (Note : orientation was not locked on the AndroidManifest file, I actually need the landscape mode elsewhere in the app.) **What is the fix?** Sometimes when the props `orientation` is set, the fragment and its activity associated are not yet available. So doing a setOrientation in the updateProps function does not work. They can also be set directly on the onUpdate function of the ScreenFragment itself, using the `trySetWindowTrait` function and the main activity; but the "super" allowing this behavior to the ScreenStackFragment (=the ones that I use) were not here. A similar fix is also done in the ScreenStackHeaderConfig `onUpdate` function. But in my case, I did not have a headerConfig: so the `trySetWindowTrait` was not called at all ... and my app could go landscape just after the opening. After adding the `super`, bug was fixed and my app was well locked in portrait mode 👌 👌 ## Screenshots / GIFs Here is the scheme that I made during my debugging session:  Here is just for fun my app going in portrait mode when it was **not** suppose to go: <image src="https://github.com/software-mansion/react-native-screens/assets/67843879/cb5860e4-9392-4257-aa3a-29d0292bffd4" width=300/> ## Test code and steps to reproduce <!-- Please include code that can be used to test this change and short description how this example should work. This snippet should be as minimal as possible and ready to be pasted into editor (don't exclude exports or remove "not important" parts of reproduction example) --> ## Checklist - [ ] Included code example that can be used to test this change - [x] Updated TS types -> no need I think - [ ] Ensured that CI passes
Description
I found this working on my app going in portrait mode on Android quite aleatory, when it was supposed to be locked on React Navigation - using the
orientation: "portrait"on my screens.(Note : orientation was not locked on the AndroidManifest file, I actually need the landscape mode elsewhere in the app.)
What is the fix?
Sometimes when the props
orientationis set, the fragment and its activity associated are not yet available. So doing a setOrientation in the updateProps function does not work.They can also be set directly on the onUpdate function of the ScreenFragment itself, using the
trySetWindowTraitfunction and the main activity; but the "super" allowing this behavior to the ScreenStackFragment (=the ones that I use) were not here.A similar fix is also done in the ScreenStackHeaderConfig
onUpdatefunction. But in my case, I did not have a headerConfig: so thetrySetWindowTraitwas not called at all ... and my app could go landscape just after the opening.After adding the
super, bug was fixed and my app was well locked in portrait mode 👌 👌Screenshots / GIFs
Here is the scheme that I made during my debugging session:
Here is just for fun my app going in portrait mode when it was not suppose to go:
Test code and steps to reproduce
Checklist